fix(integration): respect comma-separated applyTo globs in Claude/Cursor/Windsurf#1387
Conversation
…sor/Windsurf
The applyTo frontmatter is documented as 'Glob (or comma-separated globs)'
but the Claude, Cursor, and Windsurf converters emitted the comma-list as
one literal glob string. The context optimizer also fed the unsplit value
to glob/fnmatch, producing misleading 'matches no files' warnings.
Changes:
- New helper parse_apply_to() in src/apm_cli/utils/patterns.py owns the
single source of truth for comma-splitting semantics (whitespace trim,
empty/trailing/lone-comma tolerance).
- Cursor, Windsurf, and Claude converters now branch single-glob (scalar,
backwards-compatible) vs multi-glob (YAML list) when emitting native
frontmatter.
- Optimizer's _file_matches_pattern splits only top-level commas (so brace
alternation like '**/*.{css,scss}' still works), and any-segment-matches
semantics flow through unchanged caches.
- Optimizer fallback warning now names the primitive (instead of echoing
a potentially 80+ char comma-list) and only fires when zero segments
matched anything.
- _extract_intended_directory_from_pattern uses the first segment when
given a comma-list (was previously trying the whole literal as a path).
- Copilot target is intentionally untouched - it preserves applyTo
verbatim and the consuming tool splits.
Fixes #1366
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes documented applyTo behavior by treating comma-separated values as multiple globs across the Claude/Cursor/Windsurf converters and the compilation context optimizer, while preserving Copilot’s verbatim behavior.
Changes:
- Introduces
parse_apply_to()as a shared helper for trimming/splitting comma-separatedapplyTovalues. - Updates Claude/Cursor/Windsurf instruction converters to emit either a scalar (single glob) or a YAML list (multi-glob).
- Updates the context optimizer to treat comma-lists as “match any segment”, improves fallback directory inference, and reduces warning noise (names primitive instead of echoing the raw pattern).
Show a summary per file
| File | Description |
|---|---|
| src/apm_cli/utils/patterns.py | Adds shared applyTo parsing helper used by converters/optimizer. |
| src/apm_cli/integration/instruction_integrator.py | Uses shared parser to emit correct multi-glob YAML for Claude/Cursor/Windsurf. |
| src/apm_cli/compilation/context_optimizer.py | Handles comma-lists in matching/placement and improves warning messages. |
| tests/unit/utils/test_patterns.py | Adds unit coverage for parse_apply_to(). |
| tests/unit/integration/test_instruction_integrator.py | Adds converter coverage for comma-lists + Copilot verbatim regression test. |
| tests/unit/compilation/test_context_optimizer.py | Adds optimizer coverage for comma-lists and warning behavior. |
| tests/integration/test_apply_to_comma_e2e.py | Adds end-to-end test across all four targets. |
Copilot's findings
- Files reviewed: 7/7 changed files
- Comments generated: 2
| def parse_apply_to(value: str | None) -> list[str]: | ||
| """Split a primitive ``applyTo`` value into individual glob patterns. | ||
|
|
||
| The input is either a single glob (``"**/*.py"``) or a | ||
| comma-separated list (``"**/src/**,**/api/**"``). Each segment is | ||
| stripped of surrounding whitespace; empty segments are discarded so | ||
| leading, trailing, doubled-up, and lone commas are tolerated. | ||
|
|
||
| Returns an empty list for ``None``, empty, or whitespace-only input. | ||
| """ | ||
| if not value: | ||
| return [] | ||
| return [segment for segment in (part.strip() for part in value.split(",")) if segment] |
| # applyTo accepts a comma-separated list of globs; treat any | ||
| # segment match as a hit so list patterns mirror per-glob semantics. | ||
| # Only split on top-level commas - commas inside brace alternation | ||
| # (e.g. ``**/*.{css,scss}``) must stay attached for brace expansion. | ||
| if _has_top_level_comma(pattern): | ||
| segments = parse_apply_to(pattern) | ||
| return any(self._file_matches_pattern(file_path, seg) for seg in segments) |
Commas inside glob brace alternation ({a,b}) are not list separators.
Walk the string tracking brace depth; only split on top-level commas.
Adds regression tests for **/*.{css,scss},**/*.py and nested braces.
Addresses Copilot AI review feedback on PR #1387.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 1 | 2 | Solid fix. Single shared helper is correct and idiomatic. One DRY violation: _has_top_level_comma duplicates the brace-depth state machine from parse_apply_to. |
| CLI Logging Expert | 0 | 0 | 2 | New warning messages are a clear improvement: ASCII-only, name the primitive, follow compile-warning style. Two minor gaps (no fix hint, minor tense drift). |
| DevX UX Expert | 0 | 2 | 1 | Fixes a real documented user promise; correct and backwards-compatible. Two UX rough edges: warning drops pattern value; first-segment placement heuristic is silent. |
| Supply Chain Security Expert | 0 | 2 | 1 | Brace-aware split logic correct; Windsurf sanitization survives refactor. Two pre-existing injection gaps now more visible: Cursor/Claude lack newline sanitization; segments not double-quote escaped. |
| OSS Growth Hacker | 0 | 1 | 1 | Silent correctness bug fixed across all three major AI harnesses. CHANGELOG-worthy. Multi-harness parity story strengthened. |
| Doc Writer | 0 | 2 | 1 | Doc page documents comma-separated applyTo at field level but "What compiles where" table omits per-target rendering difference. No CHANGELOG entry yet. |
| Test Coverage Expert | 0 | 1 | 2 | Core unit coverage solid. Two gaps: first-segment-wins design untested when seg[0] is global; brace-alternation not exercised through optimizer comma-split guard. |
B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.
Top 5 follow-ups
- [Doc Writer / OSS Growth Hacker] Add CHANGELOG entry under
[Unreleased] > ### Fixedbefore merge -- user-visible bug fix across three harnesses with a filed issue; absence violates APM communication norms. - [Supply Chain Security Expert] Open hardening issue: newline sanitization parity and double-quote escaping across all three converters -- pre-existing, not a regression, but surfaced clearly by this PR; track with explicit scope.
- [Test Coverage Expert] Add unit test for comma-list where segment[0] is a global pattern (
**/...) so the optimizer falls back to root even when segment[1] implies a concrete directory -- documents the 'first segment wins' design choice as a regression trap. - [DevX UX Expert / CLI Logging Expert] Combine warning message:
applyTo for 'NAME' (PATTERN) matched no files -- check your applyTo glob-- satisfies both the debuggability goal (pattern visible) and the actionability goal (fix hint present). - [Python Architect / Doc Writer] Move
_has_top_level_commaintoutils/patterns.py(eliminates DRY violation) and update the "What compiles where" table to document Copilot verbatim vs YAML list per target -- two low-effort, high-clarity changes for one follow-up PR.
Architecture
classDiagram
direction TB
class parse_apply_to {
<<Pure>>
+parse_apply_to(value: str | None) list[str]
}
class _has_top_level_comma {
<<Pure>>
+_has_top_level_comma(pattern: str) bool
}
class ContextOptimizer {
<<Service>>
-_warnings: list[str]
-base_dir: Path
+_file_matches_pattern(file_path, pattern) bool
+_extract_intended_directory_from_pattern(pattern) Path|None
+_solve_placement_optimization(instruction, pattern, ...) tuple
}
class InstructionIntegrator {
<<BaseIntegrator>>
+_convert_to_cursor_rules(content) str
+_convert_to_windsurf_rules(content) str
+_convert_to_claude_rules(content) str
}
class Instruction {
<<ValueObject>>
+name: str
+apply_to: str
+file_path: Path
}
ContextOptimizer ..> parse_apply_to : imports
ContextOptimizer ..> _has_top_level_comma : imports (local copy)
ContextOptimizer ..> Instruction : reads
InstructionIntegrator ..> parse_apply_to : imports
InstructionIntegrator ..> Instruction : reads
note for _has_top_level_comma "DRY violation: brace-depth state machine duplicated from parse_apply_to"
note for parse_apply_to "Single source of truth for comma-split semantics"
class parse_apply_to:::touched
class _has_top_level_comma:::touched
class ContextOptimizer:::touched
class InstructionIntegrator:::touched
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
A(["apm compile\ncli entry"]) --> B["context_optimizer.py\n_solve_placement_optimization"]
B --> C["_file_matches_pattern\n(file_path, pattern: str)"]
C --> D{"_has_top_level_comma\n(pattern)"}
D -- "yes: comma outside braces" --> E["parse_apply_to(pattern)\n-> list[str] segments"]
E --> F["for each seg:\n_file_matches_pattern(file_path, seg)\n[recursive, single-glob path]"]
F --> G{"any segment match?"}
G -- yes --> H(["return True"])
G -- no --> I(["return False"])
D -- "no: single glob" --> J["_expand_glob_pattern(pattern)\n-> expanded_patterns"]
J --> K["fnmatch / Path.match per expanded glob"]
K --> G
B --> L["_extract_intended_directory_from_pattern\n(pattern)"]
L --> M{"_has_top_level_comma\n(pattern)"}
M -- "yes" --> N["parse_apply_to(pattern)\n-> take segments[0]"]
N --> O["analyze first segment\nonly for dir heuristic"]
M -- "no" --> O
O --> P{"starts with **/"}
P -- "yes" --> Q(["return None (global)"])
P -- "no" --> R(["return Path(segment split on /)"])
B --> S{"matching_dirs empty?"}
S -- "yes, intended_dir found" --> T["_warnings.append\napplyTo for 'name' matched no files"]
S -- "yes, no intended_dir" --> U["_warnings.append\nfallback to root"]
subgraph integrators ["instruction_integrator.py - converters"]
V["_convert_to_cursor_rules"] --> W["parse_apply_to(apply_to)"]
X["_convert_to_windsurf_rules"] --> W
Y["_convert_to_claude_rules"] --> W
W --> Z{"len(globs)"}
Z -- "1" --> AA["scalar: globs: 'pattern'"]
Z -- ">1" --> AB["YAML list:\nglobs:\n - p1\n - p2"]
end
Recommendation
Ship after adding the CHANGELOG entry -- that is the one pre-merge action required by APM's own communication norms. Everything else is a follow-up. The fix is correct, non-breaking, and closes a documented promise that was silently broken across three harnesses. The supply-chain gaps are pre-existing and do not block this PR; open a dedicated hardening issue before or alongside merge. The test and doc gaps are low-risk and can land in a follow-up PR. No panelist found a regression introduced by this change.
Full per-persona findings
Python Architect
- [recommended]
_has_top_level_commaduplicates the brace-depth state machine fromparse_apply_toatsrc/apm_cli/compilation/context_optimizer.py:33
Both functions implement the same character-walk with brace-depth tracking. The guard exists to avoid infinite recursion in_file_matches_pattern(correct reason), but the implementation is copy-pasted. If brace semantics change, both sites must be updated in sync. Fix: move_has_top_level_commaintoutils/patterns.pyand import from there. - [nit]
_extract_intended_directory_from_pattern: first-segment heuristic has no inline comment at the callsite explaining why only segment[0] is consulted.
Suggested: Add a one-line comment:# placement requires a single directory; first glob is the primary scope. - [nit]
parse_apply_todocstring has an orphaned blank line before theReturns:sentence atsrc/apm_cli/utils/patterns.py:23.
CLI Logging Expert
- [nit] No actionable fix hint in the new warning messages. APM logging convention calls for appending guidance such as
(check your applyTo glob)so the user knows where to look.
Suggested:f"applyTo for '{name}' matched no files - placing in '...' (check your applyTo glob)" - [nit] Minor tense inconsistency: new messages use past tense ("matched") while some existing warnings use present-tense forms. Low impact; track as style debt.
DevX UX Expert
- [recommended] Warning message drops the pattern value -- user cannot debug which glob(s) failed. The primitive name is good but removing the pattern string forces the user to manually inspect their frontmatter. A combined form ("applyTo for 'NAME' (PATTERN) matched no files") serves both intents at
src/apm_cli/compilation/context_optimizer.py. - [recommended] First-segment placement heuristic for comma-lists is silent and surprising. When segment[0] is a global pattern and segment[1] would yield a concrete directory, the user gets root placement with no explanation. A note in the warning or a code comment at the heuristic would give context.
- [nit] Copilot verbatim-passthrough vs YAML-list split deserves a code comment in
_convert_to_cursor_rules/_convert_to_windsurf_rulesvisible to future contributors to prevent someone from "fixing" the Copilot path.
Supply Chain Security Expert
- [recommended] Cursor and Claude converters lack newline sanitization before
parse_apply_to. Windsurf explicitly strips\n/\rfromapply_tobefore splitting; Cursor and Claude do not. A craftedapplyTovalue with an embedded newline can produce a new top-level YAML key in the target IDE config file after the segment is emitted inside a double-quoted scalar. Fix: addapply_to = apply_to.replace('\n', ' ').replace('\r', ' ').strip()in_convert_to_cursor_rulesand_convert_to_claude_rulesbefore callingparse_apply_to. - [recommended] Double-quote characters inside glob segments are not escaped before being wrapped in
"..."YAML scalars. All three converters emitf' - "{g}"'without escaping"insideg. An unescaped"terminates a YAML double-quoted scalar. Fix:g_safe = g.replace('"', '\\"')at every emit site. - [nit]
parse_apply_todocstring should note it performs no sanitization and that callers are responsible for special-character handling before using segments in formatted output.
OSS Growth Hacker
- [recommended] Missing CHANGELOG entry. This is a user-visible bug fix affecting three harnesses simultaneously, closing a documented-but-broken feature. Proposed text: "
applyTocomma-separated globs now correctly expand to per-file patterns in Claude (paths:), Cursor (globs:), and Windsurf (globs:) outputs; previously emitted as a single broken literal. (fix(integration): respect comma-separated applyTo globs in Claude/Cursor/Windsurf #1387, closes [BUG] applyTo comma-separated globs are not split ? Claude target emits broken paths: and compiler warns "matches no files" #1366)" - [nit] The
applyTofield is documented as accepting comma-separated globs but the only concrete example in the doc page shows a single glob. A two-line multi-glob example would compound this fix's impact.
Doc Writer
- [recommended] Missing CHANGELOG entry under
[Unreleased] > ### Fixed. The SSH fix (fix(deps): honor SSH user from dependency URL (closes #1383) #1385) has an entry; this fix should too. See OSS Growth Hacker for proposed text.
File:CHANGELOG.md - [recommended] "What compiles where" table omits the per-target rendering difference for comma-lists: Copilot preserves verbatim; Claude/Cursor/Windsurf expand to YAML list. Without this, a producer cannot predict what will land in each target -- exactly the class of confusion that made the bug invisible until tested. Table should extend the Transform column and include a concrete multi-glob example.
File:docs/src/content/docs/producer/author-primitives/instructions-and-agents.md - [nit] Common pitfalls section could add: "Copilot receives the raw comma string verbatim; use brace alternation (
**/*.{ts,js}) if you need multi-pattern coverage in Copilot."
Test Coverage Expert
- [recommended]
_extract_intended_directory_from_pattern: no test asserts the consequence of the "first segment wins" design when segment[0] is a global pattern but segment[1] has a concrete directory. The implementation documents the choice; no test traps it as a regression.
Evidence: missing | unit |tests/unit/compilation/test_context_optimizer.py::TestApplyToCommaInOptimizer::test_comma_list_global_first_segment_falls_back_to_root
Proves: when applyTo is a comma-list whose first segment is global, optimizer places at root even if a later segment implies a concrete directory. - [nit] Brace-alternation patterns (e.g.
**/*.{css,scss}) are not exercised through the optimizer's_has_top_level_comma/_file_matches_patternpath. If the depth-tracking regressed, no optimizer-level test would catch it.
Evidence: missing | unit |tests/unit/compilation/test_context_optimizer.py::TestApplyToCommaInOptimizer::test_brace_alternation_not_split_in_optimizer - [nit] E2E integration test at
tests/integration/test_apply_to_comma_e2e.pyuses real files and real integrator instances (integration-with-fixtures tier). Could not execute in sandbox; outcome unknown per S7 probe rule.
Auth Expert -- inactive
No auth files changed; PR only touches glob pattern parsing in context_optimizer, instruction_integrator, and a patterns utility -- no token management, credential resolution, host classification, or remote-host auth surface is involved.
This panel is advisory. It does not block merge. Re-apply the panel-review label after addressing feedback to re-run.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
pypi.org
To allow these domains, add them to the
network.allowedlist in your workflow frontmatter:
network:
allowed:
- defaults
- "pypi.org"See Network Configuration for more information.
Generated by PR Review Panel for issue #1387 · ● 3.4M · ◷
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 0 | 2 | Clean extraction of parse_apply_to into a shared utility with correct brace-depth-aware splitting; converters DRY up nicely. |
| CLI Logging Expert | 0 | 0 | 1 | Warning rewrite correctly names the primitive and suppresses raw-pattern noise; one minor wording nit. |
| DevX UX Expert | 0 | 0 | 2 | Fix closes the documented-vs-emitted gap for comma-separated applyTo across Claude/Cursor/Windsurf; Copilot asymmetry is intentional and invisible to authors. Ship. |
| Supply Chain Security | 0 | 0 | 1 | Small focused fix; no auth/lockfile/download/token surfaces; traversal still gated; brace-depth parser distinguishes list commas from brace commas. |
| OSS Growth Hacker | 0 | 1 | 1 | Solid trust-building fix; earns a CHANGELOG beat reinforcing the multi-harness fidelity claim. |
| Doc Writer | 0 | 3 | 1 | Behavior consistent with existing "What compiles where" table, but the comma-list contract is documented in one line, never demonstrated, and the CHANGELOG/apm-usage resource have no entry. |
| Test Coverage Expert | 0 | 0 | 1 | All four critical surfaces covered; 34 tests pass in 0.70s. Brace-with-comma e2e scenario is nit-level gap. |
B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.
Top 5 follow-ups
- [Doc Writer] Add
CHANGELOG.mdFixed entry under [Unreleased] referencing fix(integration): respect comma-separated applyTo globs in Claude/Cursor/Windsurf #1387/[BUG] applyTo comma-separated globs are not split ? Claude target emits broken paths: and compiler warns "matches no files" #1366. -- Two panelists converge; the public record of community-reported fixes is how trust compounds. Without this entry the fix is invisible to changelog readers. - [Doc Writer] Add comma-list
applyToexample indocs/src/content/docs/producer/author-primitives/instructions-and-agents.md. -- The comma-list contract is documented in one line but never demonstrated; authors will hit the same confusion without an example. - [Doc Writer] Update
packages/apm-guide/.apm/skills/apm-usage/package-authoring.mdwith comma-list syntax. -- Linting rule 4 requires resource-file parity when primitive formats change; this is a format clarification that the apm-usage resource currently omits. - [Supply Chain Security] Sanitise glob segments containing literal quotes before f-string YAML emission in
instruction_integrator.py. -- Defence-in-depth against YAML injection; risk is LOW today (consumers reject malformed YAML) but the producer should not emit it. - [Python Architect] Move
_has_top_level_commahelper fromcontext_optimizer.pyintoutils/patterns.pyalongsideparse_apply_to. -- Eliminates duplicated brace-depth logic; single source of truth for comma-vs-brace discrimination.
Architecture
classDiagram
direction LR
class ContextOptimizer {
+optimize_instruction_placement(instructions)
-_file_matches_pattern(file_path, pattern) bool
-_extract_intended_directory_from_pattern(pattern) Path
-_expand_glob_pattern(pattern) list
-_cached_glob(pattern) list
-_warnings list
}
class InstructionIntegrator {
<<BaseIntegrator>>
+copy_instruction(src, dst)
+copy_instruction_cursor(src, dst)
+copy_instruction_windsurf(src, dst)
+copy_instruction_claude(src, dst)
-_convert_to_cursor_rules(content) str
-_convert_to_windsurf_rules(content) str
-_convert_to_claude_rules(content) str
}
class BaseIntegrator {
<<Abstract>>
}
class parse_apply_to {
<<Pure>>
+parse_apply_to(value) list~str~
}
class _has_top_level_comma {
<<Pure>>
+_has_top_level_comma(pattern) bool
}
class Instruction {
<<Dataclass>>
+name str
+apply_to str
+file_path Path
+content str
}
InstructionIntegrator --|> BaseIntegrator
InstructionIntegrator ..> parse_apply_to : calls at emit
ContextOptimizer ..> _has_top_level_comma : guards split
ContextOptimizer ..> parse_apply_to : splits pattern
ContextOptimizer ..> Instruction : reads apply_to
InstructionIntegrator ..> Instruction : reads apply_to
class parse_apply_to:::touched
class _has_top_level_comma:::touched
class InstructionIntegrator:::touched
class ContextOptimizer:::touched
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
A["apm compile / apm install<br/>CLI entry"] --> B{"Target format?"}
B -->|Copilot| C["copy_instruction<br/>preserves applyTo verbatim"]
B -->|Cursor| D["_convert_to_cursor_rules<br/>instruction_integrator.py"]
B -->|Windsurf| E["_convert_to_windsurf_rules<br/>instruction_integrator.py"]
B -->|Claude| F["_convert_to_claude_rules<br/>instruction_integrator.py"]
D --> G["parse_apply_to(apply_to)<br/>utils/patterns.py"]
E --> G
F --> G
G --> H{"len(globs) == 1?"}
H -->|yes| I["emit scalar: globs/paths: glob"]
H -->|>1| J["emit YAML list under globs/paths"]
H -->|0 empty| K["skip key or trigger: always_on"]
A --> L["ContextOptimizer.optimize_instruction_placement"]
L --> M["_file_matches_pattern(file, pattern)<br/>context_optimizer.py"]
M --> N{"_has_top_level_comma(pattern)?"}
N -->|no| O["_expand_glob_pattern<br/>then fnmatch / cached glob"]
N -->|yes| P["parse_apply_to(pattern)"]
P --> Q["any segment matches<br/>recursive per-segment"]
L --> R["_extract_intended_directory_from_pattern"]
R --> S{"_has_top_level_comma?"}
S -->|yes| T["parse_apply_to -> use segments[0]"]
S -->|no| U["existing first-part heuristic"]
Recommendation
Merge now. The bug-fix correctness is solid -- 8762 existing tests plus 34 new unit/integration/e2e tests all green, and the brace-with-comma interaction is explicitly proven by test_brace_alternation_mixed_with_top_level_comma. Track the CHANGELOG + docs-example pair as a same-day follow-up (can land in a tiny docs-only PR or be amended before release tag); the remaining three follow-ups are housekeeping that can land at leisure.
Full per-persona findings
Python Architect
- [nit]
_has_top_level_commaduplicates brace-depth logic atsrc/apm_cli/compilation/context_optimizer.py:30
Move the helper toutils/patterns.pyalongsideparse_apply_toso both the check and the parse live in the same module. The performance justification (avoids list alloc in_file_matches_patternhot path) is valid, but the logic should still be co-located with the canonical parser. - [nit]
_extract_intended_directory_from_patternonly consults the first segment of a comma-list atsrc/apm_cli/compilation/context_optimizer.py:685
If the first segment is a global pattern (e.g.**/*.py,docs/**/*.md), the method returnsNoneand loses the usefuldocs/signal from the second segment. Consider iterating segments until one yields a non-Nonedirectory. Low priority -- current behavior is safe (falls back to root).
CLI Logging Expert
- [nit] Warning wording
applyTo for X matched no filesuses an attribute name as the sentence subject atsrc/apm_cli/compilation/context_optimizer.py
Message Writing Rule Will there be MCP coverage? #3 says "Name the thing" -- the user cares about the primitive, not the YAML key. Consider:"'{name}' matched no files -- placing at project root". The primitive name IS the thing;applyToadds nothing the user can act on.
DevX UX Expert
- [nit] Warning message lost the raw pattern at
src/apm_cli/compilation/context_optimizer.py:594
The old warning let an author grep for the offending pattern. The new wording is friendlier but loses the pattern entirely. In verbose/diagnostic mode it would help to echo the first segment or the count of segments so the author knows roughly why. Low priority because the name alone is usually sufficient for lookup. - [nit] Cursor single-glob scalar vs multi-glob list asymmetry is correct but undocumented in the harness table at
docs/src/content/docs/producer/author-primitives/instructions-and-agents.md:77
Both YAML shapes are accepted; a one-word annotation (scalar or list) would prevent future contributor confusion. Purely editorial.
Supply Chain Security
- [nit] YAML injection defence-in-depth via unescaped double-quote in glob segment at
src/apm_cli/integration/instruction_integrator.py:122
A glob segment containing a literal"(afterparse_apply_to) would produce broken or injectable YAML through thef' - "{g}"'emission. Adding.replace('"', '\\"')(or using a proper YAML serializer) on each segment before interpolation would close this. Risk is LOW (consuming tools likely reject malformed YAML), but defence-in-depth says sanitise at the producer.
OSS Growth Hacker
- [recommended] CHANGELOG entry should frame fix as multi-harness fidelity proof, not just a bug fix
The "one manifest, many harnesses" claim is load-bearing for APM positioning. The CHANGELOG entry should lead with the user benefit and reference [BUG] applyTo comma-separated globs are not split ? Claude target emits broken paths: and compiler warns "matches no files" #1366 as community-reported -- this credits the contributor funnel and signals responsiveness.
Suggested:Fixed: comma-separated applyTo globs now emit native multi-glob shapes in Claude, Cursor, and Windsurf targets (#1366). One manifest definition, correct behavior everywhere -- as documented. - [nit] Warning message improvement is a hidden DevX win worth calling out in release notes at
src/apm_cli/compilation/context_optimizer.py:593
ChangingPattern <raw-glob-list> matches no filestoapplyTo for <name> matched no filesis a small but meaningful friction reduction for new primitive authors debugging placement. Worth a bullet under "improved diagnostics".
Auth Expert -- inactive
PR touches only compilation/context_optimizer.py, integration/instruction_integrator.py, utils/patterns.py, and their tests -- purely applyTo glob-splitting logic with no auth, token, credential, or host-classification changes.
Doc Writer
- [recommended] Missing
CHANGELOG.mdFixed entry for fix(integration): respect comma-separated applyTo globs in Claude/Cursor/Windsurf #1387/[BUG] applyTo comma-separated globs are not split ? Claude target emits broken paths: and compiler warns "matches no files" #1366
PR 1387 fixes a user-visible bug; the## [Unreleased] ### Fixedblock lists other entries but no entry for this PR.
Suggested: Add a one-line Fixed entry under [Unreleased]:Claude, Cursor, and Windsurf instruction rules now honor comma-separated applyTo globs as native multi-entry paths: / globs: lists instead of a single joined string; Copilot is unchanged. (#1387, closes #1366). - [recommended] Missing comma-list example in
docs/src/content/docs/producer/author-primitives/instructions-and-agents.md
TheapplyTorow already says "Glob (or comma-separated globs)", but the only Frontmatter example shows a single glob. The load-bearing comma-list semantic this PR makes correct across three harnesses is never demonstrated.
Suggested: Add or extend an example:applyTo: "src/**/*.py, tests/**/*.py" # comma-separated -> native list per target. - [recommended] Update
packages/apm-guide/.apm/skills/apm-usage/package-authoring.mdInstruction example with comma-list form
This is the agent-runtime resource consulted when an agent authors a primitive. The current example shows onlyapplyTo: "**/*.py". Per the apm-usage dependency contract (linting rule 4), primitive-format changes must update this resource.
Suggested:applyTo: "src/**/*.py, tests/**/*.py" # single glob or comma-separated list. - [nit] "What compiles where" table at
docs/src/content/docs/producer/author-primitives/instructions-and-agents.md:76-78is already correct post-fix
No doc change needed; the existingpaths:list /globs:transform descriptions remain accurate; the Copilot row ("verbatim; applyTo preserved") is also still accurate. Noting here for reviewer clarity.
Test Coverage Expert
- [nit] E2e test lacks a brace-expansion-mixed-with-comma scenario at
tests/integration/test_apply_to_comma_e2e.py
The e2e test exercises only plain comma-separated globs. The tricky brace-vs-comma interaction is unit-tested (passed) attests/unit/utils/test_patterns.py::TestParseApplyTo::test_brace_alternation_mixed_with_top_level_comma, and converters delegate directly toparse_apply_to, so this is defence-in-depth polish rather than a missing regression trap.
Suggested: Add a parametrized case withapplyTo='**/*.{css,scss},**/*.py'asserting Cursor/Claude/Windsurf emit two YAML list entries and the brace group stays intact.
Proof (passed):tests/unit/utils/test_patterns.py::TestParseApplyTo::test_brace_alternation_mixed_with_top_level_comma-- proves: brace-expansion commas inside{...}are not treated as list separators when mixed with top-level commas [multi-harness-support, devx]
assert parse_apply_to("**/*.{css,scss},**/*.py") == ["**/*.{css,scss}", "**/*.py"]
Performance Expert -- inactive
PR touches only compilation/context_optimizer.py, integration/instruction_integrator.py, and utils/patterns.py -- compile-time glob splitting and warning text, no install/download/transport/cache hot path affected.
This panel is advisory. It does not block merge. Re-apply the panel-review label after addressing feedback to re-run.
Panel follow-ups landed in #1449PR #1387 is already merged; the 5 nit-level items from the Items folded (5/5):
Items deferred: none. Validation:
Follow-up PR: #1449 |
…1449) Lands the panel recommendations on #1387 (closes #1366) that were filed as nits after the merge: - CHANGELOG: add Fixed entry under [Unreleased] documenting the comma-separated applyTo fix across Claude/Cursor/Windsurf, with the Copilot-verbatim carve-out spelled out (#1387, closes #1366). - docs(instructions-and-agents): demonstrate the comma-list contract with a worked example, document the brace-alternation carve-out, and clarify per-target rendering in "What compiles where". - docs(apm-usage/package-authoring): mirror the comma-list syntax note in the apm-usage skill resource (linting rule 4). - refactor(patterns): move _has_top_level_comma from context_optimizer into utils/patterns.py as has_top_level_comma (DRY: single source of truth for the brace-depth state machine). - security(integrators): emit YAML scalars via a new yaml_double_quote() helper that escapes backslashes, quotes, and control characters. Defence-in-depth against adversarial or copy-paste-mangled applyTo values; risk is LOW today (parse_apply_to strips whitespace and Windsurf strips newlines) but producer output is now well-formed even on hostile input. Added 11 unit tests including yaml.safe_load round-trip and a has_top_level_comma brace-aware suite. Lint silent; 175 tests pass (26 in test_patterns.py). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
) * feat(batch-bug-shepherd): operator visibility + fold-in invariant Refactors the batch-bug-shepherd skill to address two genesis-validated blockers and ship two missing capabilities discovered during a real sweep-all run over 14 microsoft/apm bugs: 1. OPERATOR VISIBILITY (was: silent 30-minute fan-outs) - New asset assets/progress-diagram.md: mermaid template + 5-state palette (pending/active/done/blocked/skipped) + per-phase render rules + dispatch-table contract. - SKILL.md adds 'Operator visibility is a contract' invariant; each phase boundary re-renders the diagram with current-phase coloring and prints a subagent_id -> target dispatch table BEFORE fan-out. - Operator can follow long sagas at a glance instead of waiting in the dark for the next checkpoint. 2. FOLD-IN INVARIANT (was: panel recommendations silently dropped) - assets/verdict-schema.json: shepherd_return gains required recommended_followups[] channel; completion_return gains folded_followups[] + deferred_followups[]; extracted reusable followup_item definition. - assets/shepherd-prompt.md: fixed verdict mapping bug (ship_with_followups + 0 blocking -> ready-to-merge, not needs-author-changes); added recommended_followups extraction step with required source_persona + optional fold_hint tagging. - assets/completion-prompt.md: full rewrite. Adds RECOMMENDED_FOLLOWUPS input; encodes FOLD vs DEFER classifier (FOLD: touches diff / single helper / regression trap / hermetic test / inline comment; DEFER: cross-cutting refactor / new feature / broad doc / architectural addition); per-FOLD item consultation with source_persona + python-architect lens; DEFER items filed as gh issue create tracking issues (never silently dropped); mid-flight reclassify rule to avoid stalls. - SKILL.md adds 'Bias toward folding recommended items' invariant and rewrites Phase 4 spawn contract (9 steps) to thread the recommended_followups channel end-to-end. Eval gate - +3 rubric anchors per content fixture (progress-diagram-header, mermaid-flowchart-rendered, dispatch-table-before-fanout) and +3 invariant anchors (recommended-followups-channel, fold-defer-classifier, tracking-issue-for-defer). - All 12 new anchors MATCH with_skill fixtures and MISS without_skill fixtures (clean value delta). - +3 no-fire trigger items for single-PR fold-in phrasing so the dispatcher will not misfire the batch outer-loop on single-PR fold work (e.g. 'fold the panel recommendations into PR #1234' remains apm-review-panel completion territory). Validation - Schema validates via jsonschema Draft7; accepts new shapes, rejects shepherd_return missing recommended_followups[]. - SKILL.md: 367 lines / 4483 tokens (caps: 500 / 5000). - Description: 965 / 1024 chars; mentions FOLD invariant. - 0 non-ASCII bytes across all modified files. - All 4 changed JSON files parse. Real-task evidence (this skill iteration was driven by a live run) - 5 of 6 in-flight community PRs had their panel recommendations folded in-PR by completion subagents following the new contract, yielding 22 folded items and 8 deferred-to-tracking items across PRs #1387, #1396, #1441, #1443, #1444. The 6th (#1442) is in flight as this lands. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * batch-bug-shepherd: add Phase 5 mergeability gate Adds a post-wave gate that re-probes mergeability for every PR the saga marked ready-to-merge, dispatches one conflict-resolution subagent per CONFLICTING PR, and partitions returns into four post-gate statuses before the final report claims anything is mergeable. Mergeability is post-wave truth, not pre-wave assumption: a PR that Phase 4 marked ready can stop being mergeable the moment the maintainer lands another PR onto main. Without this gate the report ships stale ready claims. Design driven through the genesis skill end-to-end (steps 1-6 handoff packet, steps 7a-7b coder pass, step 8 validation): - NEW Phase 5 (mergeability gate) between completion (Phase 4) and renamed final report (Phase 5 -> Phase 6). - Sub-phases 5a probe (read-only, single-thread, gh pr view --json mergeStateStatus), 5b fan-out (one conflict-resolution subagent per CONFLICTING PR), 5c trust-but-verify re-probe + four-way partition (resolved / requires-author-action / requires-human-judgment / resolution-failed). - NEW assets/conflict-resolution-prompt.md spawn body for 5b. Encodes rebase, faithful merge of both intents, mutation-break re-check, lint silent, --force-with-lease push, re-probe, resolution-confirmation comment. - NEW references/mergeability-gate.md load-on-demand orchestrator step-by-step (load trigger: WHEN ENTERING PHASE 5). Keeps SKILL.md under 5000-token budget. - Schema extends verdict-schema.json oneOf with conflict_resolution_return; --force-with-lease enforced via regex pattern guard on push_command field; bare --force rejected. Five rejection cases validated. - Two-comment-per-PR cap as new architecture invariant: at most one completion-confirmation (Phase 4) + one resolution-confirmation (Phase 5b) per PR. - Progress diagram extended with WAVE4 subgraph (P5a/P5b/P5c), skipped-state semantics, P5b dispatch table requirement. - Final report extended with three new partition sections plus a RESOLUTION CONFIRMATION COMMENT block and mergeability-gate disciplines line. - Evals: +3 content rubric anchors (mergeability-probe-cli, force-with-lease-on-push, post-wave-partition-columns) + 1 optional anchor (two-comment-cap); +1 fire + 2 no-fire trigger items; fixture diff shows the gate firing on a sweep with 2 conflicting PRs and the without-skill failure mode (stale ready claim). SKILL.md: 388 lines / 4867 tokens (budget 500/5000). ASCII only. CI lint pair silent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(bbs): add Phase 1.5 strategic-alignment gate + PRINCIPLES.md Adds a new wave between Phase 1 (triage) and Phase 2 (PR-in-flight cross-reference) that checks every LEGIT bug against the project's rejection contract before spending shepherd / fix / completion work on it. What changes: - NEW PRINCIPLES.md at repo root: 7 numbered principles encoding the project's hard nos (P1 no invented frontmatter; P2 multi-harness with traction gating; P3 vendor neutral; P4 UX floor is not a trade) plus 3 supporting principles (P5 portability; P6 reliability over magic; P7 community over feature count). Bound to apm-ceo + bbs Phase 1.5 + apm-triage-panel + apm-review-panel as the rejection contract. - NEW bbs Phase 1.5 strategic-alignment gate (WAVE 1.5): - one apm-ceo subagent per LEGIT row, in parallel - 4-state verdict: aligned | aligned-with-reservations | out-of-scope | wrong-direction - schema-validated returns; FAILS OPEN on infrastructure failure (malformed-x2 or non-citable principle) so legit bugs are never silently demoted under gate breakage - ABORTS only when apm-ceo.agent.md or PRINCIPLES.md itself is missing (operator-actionable error) - demoted rows flip to status triaged-deferred and SKIP Phase 2/3/4/5; surface in Phase 6 under 'Recommend close as out-of-scope' partition - aligned-with-reservations rows stay in saga; downstream phases surface the reservations in review prose - deferred-PR strategic-rejection comment subagent (S7+S4+A9) posts a courtesy comment on any open PR whose underlying issue was demoted, using the would-be Phase-4 completion-comment slot (two-comments-per-PR cap preserved) - Verdict schema extended with 5th oneOf member strategic_alignment_return (kind, issue, verdict, cited_principle, rationale, reservations). - Ground-truth table grows two columns (strategic_verdict + strategic_rationale) and one status value (triaged-deferred). - Progress diagram inserts P15 between P1 and P2; dispatch-table contract extends to Phase 1.5. - Final-report template adds 'Recommend close as out-of-scope' partition and 'Aligned with reservations' surfacing section. - 2 new fire trigger evals + 1 no-fire (PRINCIPLES.md authoring guard) + 1 new rubric anchor on the three-issues-mixed scenario. Genesis design artifact lives in the session plan store; SKILL.md body remains within 500-line / 5000-token budget (406 lines / 4943 tokens after trimming pre-existing verbose passages on operator-visibility, mergeability, fold-in, composition, and operating-contract sections to make room). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: danielmeppiel <danielmeppiel@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes #1366
WHY
applyTofrontmatter is documented as "Glob (or comma-separated globs)" indocs/src/content/docs/producer/author-primitives/instructions-and-agents.md,but the Claude, Cursor, and Windsurf converters emit comma-list values as a
single literal glob string, and the context optimizer feeds the unsplit value
to glob/fnmatch (producing misleading
Pattern '...' matches no fileswarnings).Copilot was already correct because its converter preserves
applyToverbatimand the consuming tool splits — that path is intentionally unchanged.
WHAT
parse_apply_to(value) -> list[str]insrc/apm_cli/utils/patterns.pyowns the canonical comma-split semantics (whitespace trim, empty / trailing /
leading / lone-comma tolerance).
_convert_to_cursor_rules,_convert_to_windsurf_rules, and_convert_to_claude_rulesnow call the helper and branch:context_optimizer._file_matches_patternsplits only top-level commas(so brace alternation like
**/*.{css,scss}is unaffected) and treats anysegment match as a hit.
_extract_intended_directory_from_patternconsults the first segment whengiven a comma-list (previously it tried the whole literal as a path).
echoing the (potentially 80+ char) raw pattern, and still fires exactly
once per primitive — not once per comma segment.
Design summary
Picked python-architect option (a): single shared helper instead of per-converter
splits, so the comma-split contract (trim, drop empties) is defined once. Kept
Instruction.apply_to: strbecause it is used as a dict key intemplate_builder.pyand a set member inclaude_formatter.py; converting tolist[str]would have broken hashability and silently changed grouping.Considered alternatives:
applyToonce at primitive-load time inparser.py. Rejected:forces the type change above and changes serialisation everywhere; only the
three converters and the optimizer actually need split form.
rules and drifts.
[[rules]]blocks. Rejected: notrequired by Cursor's
gray-matterparser — YAML list is accepted and farless invasive.
Validation evidence
Original repro from the issue: primitive with
applyTo: '**/src/**,**/api/**,**/services/**'compiled to all four targets:Optimizer on the same primitive (no misleading warning):
Local verification gate:
uv run --extra dev ruff check src/ tests/— silentuv run --extra dev ruff format --check src/ tests/— silentuv run --extra dev pytest tests/unit— 8762 passed (1 pre-existingunrelated failure:
test_runtime_windows::test_execute_runtime_command_uses_shlex_on_unix— environment-specific codex path resolution, exists on
main)tests/integration/test_apply_to_comma_e2e.py) — greenTestApplyToCommaInOptimizer) — greenTestApplyToCommaSplitting) — greentests/unit/utils/test_patterns.py) — greenScope note
The Copilot target is intentionally untouched in this fix — it copies the
instruction verbatim, and the consuming tool handles the comma split. The
monolithic AGENTS.md / GEMINI.md compilers (
agents_compiler.py,distributed_compiler.py,claude_formatter.pyfor AGENTS injection) arealso out of scope; the issue specifically calls out the per-file Claude /
Cursor / Windsurf converters and the placement optimizer.